Skip to content

Upgrade React to 17.0.2#306

Open
ankit0504 wants to merge 5 commits intoLanguage-Mapping:masterfrom
ankit0504:agupta/upgrade-phase-3-react-17
Open

Upgrade React to 17.0.2#306
ankit0504 wants to merge 5 commits intoLanguage-Mapping:masterfrom
ankit0504:agupta/upgrade-phase-3-react-17

Conversation

@ankit0504
Copy link
Copy Markdown
Collaborator

⚠️ Stacked on #304 (phase 1) and #305 (phase 2).

Because this repo uses a forking workflow and I can't push a base branch to upstream, this PR targets `master` directly, which means the diff temporarily includes phases 1–2's commits. Once those merge to `master`, GitHub will automatically recompute this PR's diff to show only the phase 3 change (2 files: `package.json` + `yarn.lock`).

Please review and land #304 and #305 first.

Issues resolved by this pull request

None — phase 3 of the staged upgrade. Sets up the eventual React 18 move without forcing compatibility work against libraries (MUI v4, `react-swipeable-views`) that will be replaced in later phases.

Prerequisites

  • `yarn install` after checkout — both `react` and `react-dom` major-bump, plus type packages. Peer-dep warnings on install are expected and can be ignored (see below).
  • No `.env`, Vite, or Netlify config changes.

Review type

  • FYI: dep bumps only; no API surface changes in React 17 that affect this project's code, and no component files touched.
  • Content/copy
  • Needs feedback
  • Functionality
  • Code

What's in this PR

  • `react` / `react-dom`: `^16.14.0` → `^17.0.2`.
  • `@types/react` / `@types/react-dom`: bumped to latest 17.x.
  • `@types/react-router-dom`: `^5.1.5` → `^5.3.3`. The older types were written against React 16 and triggered `TS2786 'RouterLink' cannot be used as a JSX component` across 18 files once the top-level React types moved to 17.
  • `resolutions` in `package.json`: add `@types/react` and `@types/react-dom` entries. Without this, eight transitive type packages (`@types/styled-jsx`, `@types/react-transition-group`, `@types/react-swipeable-views`, `@types/react-map-gl`, `@types/react-window`, `@types/react-router-dom`, `@types/react-router`, and one under `@types/react`'s own `csstype`) each brought their own `@types/react@16`, making tsc see multiple React type identities and fail to reconcile JSX return types. Forcing a single version eliminates the cross-package confusion.

Why not React 18 in this PR

React 18 conflicts with libraries slated for replacement in phase 4:

  • `react-swipeable-views` is archived upstream and breaks in React 18 StrictMode (which `src/index.tsx` enables). The app would need StrictMode disabled, which removes a useful developer-time safety net, or the library swapped — scope creep into phase 4.
  • `@material-ui/core` v4 has React 18 peer-dep conflicts. It largely works at runtime but is unsupported.
  • `material-table@1.68` is abandoned; its React-18 compatibility is unverified.

Landing React 17 first keeps this PR truly trivial (2 files) and de-risks the React 18 step by letting it follow MUI v5.

What to review

  • `yarn install` succeeds.
  • `npx tsc --noEmit` passes with 0 errors.
  • `yarn build` produces a successful bundle.
  • `yarn start` boots the dev server without errors at `http://localhost:3000\`.
  • No new DevTools console errors beyond the pre-existing Airtable 403 warnings.

Verification done locally

  • `npx tsc --noEmit` — 0 errors.
  • `yarn build` — passes in ~5.5s.
  • Dev server starts and serves HTTP 200.

Known-ignorable peer-dep warnings on install

Expected; each of these works with React 17 in practice despite declaring React 16 as their max:
`react-query@2`, `react-share@4`, `react-swipeable-views`, `react-window`, `react-router-dom > mini-create-react-context`.

What's next

  • Phase 4: Material-UI v4 → MUI v5 + replace `material-table` and `react-swipeable-views`.
  • Phase 5: React 17 → 18 (`createRoot`, strict-mode audit) — now unblocked by phase 4.
  • Phase 6: Router v5 → v6, react-query v2 → TanStack Query v5, react-map-gl 5 → 7, replace `react-map-gl-geocoder`.

Replace react-scripts 3.4.1 with Vite 6 + @vitejs/plugin-react. This
drops the NODE_OPTIONS=--openssl-legacy-provider workaround required
for Node 17+ on CRA, and shrinks cold dev start to ~125ms and prod
build to ~5s (from ~33s).

Changes:
- Swap scripts: yarn start -> vite; yarn build -> tsc --noEmit &&
  vite build; yarn preview -> vite preview; drop eject/test.
- Replace CRA deps with vite, @vitejs/plugin-react, vite-plugin-svgr,
  vite-tsconfig-paths.
- Add vite.config.ts configuring the React and SVGR plugins, the
  REACT_APP_ env prefix (keeps .env working as-is, with VITE_ also
  accepted), port 3000, and build output to build/ (preserving the
  Netlify publish dir).
- Move public/index.html to project root. Replace %PUBLIC_URL%/ with
  / and add the Vite module entry script.
- Remove the CRA eslintConfig stanza in package.json; real config
  lives in .eslintrc.
- Add a direct eslint@^7.32.0 devDep (previously hoisted by CRA),
  drop the react-app extends that no longer resolves, and remove the
  deleted serviceWorker.ts path from ignorePatterns.
- Delete src/serviceWorker.ts and its import; it was already calling
  unregister() only, so removal is a no-op at runtime.
- Rewrite six process.env.REACT_APP_* reads as import.meta.env.
- Change Logo.tsx SVG import to use the vite-plugin-svgr ?react
  suffix.
- Update tsconfig.json: target ES2020, add vite/client and
  vite-plugin-svgr/client types, exclude *.test.ts/tsx files from
  the build since there is no test runner anymore.
- Delete src/setupTests.ts (orphaned CRA jest setup).

Known follow-ups (deliberately out of scope for this PR):
- @mapbox/mapbox-gl-geocoder imports the Node events module, which
  Vite externalizes for the browser. If the geocoder search breaks at
  runtime, it can be addressed by swapping react-map-gl-geocoder for
  a direct mapbox-gl-geocoder integration in a later phase.
- yarn lint surfaces 55 pre-existing errors across the codebase that
  were previously masked by CRA's react-app eslint preset. These are
  unrelated to the build migration and should be addressed in a
  follow-up. Pre-commit lint-staged only runs against staged files,
  so new commits are unaffected.
Removing react-scripts in the previous commit unmasked pre-existing
eslint errors that CRA's react-app preset had been silencing.

Cleanup by category:
- 40 @typescript-eslint/no-unused-vars: set { args: "none" } in the
  rule config. These were all unused callback parameters (mostly
  createStyles theme args and event/props args in handlers) required
  by library type signatures. The project-level policy is to not
  enforce unused function arguments; genuine unused variables and
  imports are still checked.
- 5 @typescript-eslint/no-use-before-define: reorder the five helper
  function definitions so each precedes its call site. Affected:
  createLayerStyles (hooks.points.ts), handleClose (SplitCrumbs.tsx),
  scrollToTop (ResultsTable.tsx), clearFiltersBtnClick
  (ResultsToolbar.tsx), excludeUTFtext (exporting.ts).
- 6 jsx-a11y/anchor-is-valid: five MUI <Link> usages that are
  semantically buttons now use component="button" and drop the dummy
  href="#" plus its e.preventDefault() plumbing. The sixth
  (MapOptionsMenu) already uses component={Button}. All six get an
  eslint-disable-next-line comment because the rule inspects JSX
  element names and doesn't analyze MUI's component prop -- the
  markup is correct, the rule is a false positive.
- 3 parser errors in excluded test files: add src/**/*.test.ts(x) to
  .eslintrc ignorePatterns. The tsconfig already excludes these from
  the build.
@ankit0504 ankit0504 marked this pull request as ready for review April 21, 2026 19:46
The airtable package references process.env at runtime, which
webpack/CRA provided but Vite does not. Defining an empty object
prevents the ReferenceError without leaking server-side env vars.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ankit0504 ankit0504 force-pushed the agupta/upgrade-phase-3-react-17 branch from f256435 to dc3d406 Compare April 22, 2026 03:53
ankit0504 and others added 2 commits April 26, 2026 20:35
- Rename vite.config.ts to vite.config.mts for ESM compatibility
- Replace MUI Link-as-button with Button in 6 components
- Pass Airtable API key explicitly instead of relying on process.env
- Migrate remaining process.env reference to import.meta.env

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Bump react/react-dom to 17.0.2 and @types/react/@types/react-dom to
the latest 17.x. Also bump @types/react-router-dom to 5.3.3 (5.1.5
was typed against React 16 and triggered TS2786 'cannot be used as
a JSX component' errors for RouterLink passed to MUI component
props).

Add @types/react and @types/react-dom to the resolutions field so
that transitively-installed type packages (eight of them — MUI,
react-map-gl, react-swipeable-views, etc. all vendored their own
@types/react@16) do not conflict with the top-level version. Without
this, tsc sees multiple React type identities and cannot reconcile
JSX element returns across package boundaries.

React 17 has no API surface changes that affect this project, so no
component code changes are required. React 18 is intentionally
deferred to a later phase, after MUI v5 lands — react-swipeable-views
is archived and breaks in React 18 StrictMode, and MUI v4 has React
18 peer-dep conflicts.

Known-ignorable peer-dep warnings on install: react-query@2,
react-share@4, react-swipeable-views, react-window, and
react-router-dom all still declare React 16 as their max in their
peer range, but each of them works with React 17 in practice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant